Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ (go/v4): Add method to allow users distribute the project #3626

Merged
merged 1 commit into from
Jan 26, 2024
Merged

✨ (go/v4): Add method to allow users distribute the project #3626

merged 1 commit into from
Jan 26, 2024

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Sep 17, 2023 •
edited
Loading

Description

  • Add target to build the installer under dist/install.yaml
  • Also, add e2e tests to ensure that installer will be working well as expected
  • By last, add instruction into the README generate to make clear how to achieve this goal

Proposed Solution

Just generate the YAML with all that should be installed (same kustomize build of make install and make deploy) so that the project can be distributed as usually it is done by many solutions like Operator Prometheus for example, where users can run: kubectl apply -f dist/install.yaml and maintainers can easily project the raw of the file.

Motivation

Closes: #3185

IMPORTANT: For we are able to check out the changes of this PR we must to have here the fix addressed in the PR: #3627 . So this one is blocked until we are able to merge the fix

/hold

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 17, 2023
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 17, 2023
@camilamacedo86 camilamacedo86 changed the title Add method to allow users distribute the project ✨ Add method to allow users distribute the project Sep 17, 2023
@camilamacedo86 camilamacedo86 force-pushed the add-distro-mode branch 2 times, most recently from f253a26 to 5c26bfb Compare September 17, 2023 06:52
@camilamacedo86 camilamacedo86 changed the title ✨ Add method to allow users distribute the project ✨ (go/v4): Add method to allow users distribute the project Sep 17, 2023
@camilamacedo86 camilamacedo86 changed the title ✨ (go/v4): Add method to allow users distribute the project ✨ (WIP) - (go/v4): Add method to allow users distribute the project Sep 17, 2023
@camilamacedo86 camilamacedo86 force-pushed the add-distro-mode branch 2 times, most recently from 77fbe31 to f97ec3f Compare September 17, 2023 10:42
@camilamacedo86 camilamacedo86 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2023
@camilamacedo86 camilamacedo86 force-pushed the add-distro-mode branch 4 times, most recently from 582d60b to e1b428b Compare September 19, 2023 06:24
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 26, 2023
@camilamacedo86 camilamacedo86 force-pushed the add-distro-mode branch 2 times, most recently from e9e1ead to 6017d70 Compare January 18, 2024 20:58
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2024
@camilamacedo86 camilamacedo86 force-pushed the add-distro-mode branch 2 times, most recently from 7c4353f to 0e22981 Compare January 22, 2024 11:46
@camilamacedo86 camilamacedo86 changed the title ✨ (WIP) - (go/v4): Add method to allow users distribute the project ✨ (go/v4): Add method to allow users distribute the project Jan 22, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 22, 2024
@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Jan 22, 2024 •
edited
Loading

HI @dashanji, @Kavinjsir , @Jay-Madden, @LCaparelli, @vbehar, @varshaprasad96 @everettraven

This one seems good to fly now.
Feel free to check.

Also, see that we are testing the installer with the e2e tests.

/hold
(To just give the opportunity for more people be able to check out and LGTM)

@camilamacedo86
Copy link
Member Author

Removing the
/hold cancel

Kavinjsir
Kavinjsir reviewed Jan 24, 2024
View reviewed changes
Copy link
Contributor

@Kavinjsir Kavinjsir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me in general!
This is a great feature.
I just left some nit and one open question.

pkg/plugins/golang/v4/scaffolds/internal/templates/readme.go Outdated Show resolved Hide resolved
pkg/plugins/golang/v4/scaffolds/internal/templates/readme.go Outdated Show resolved Hide resolved
pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go Show resolved Hide resolved
pkg/plugins/golang/v4/scaffolds/internal/templates/readme.go Show resolved Hide resolved
camilamacedo86
camilamacedo86 commented Jan 24, 2024
View reviewed changes
Copy link
Member Author

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kavinjsir

I just remembered that I need to push the changes.
Now we have them with your suggestions !!!

I appreciate a lot your help in review.
Please let me know what you think now.

Kavinjsir
Kavinjsir approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@Kavinjsir Kavinjsir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@dashanji
Copy link
Contributor

@camilamacedo86
Thanks for the great feature. I think it's a good point to build a bundled YAML for distributing the kubebuilder operator.
I just wonder what's the difference between kubectl apply -f https://raw.githubusercontent.com/<org>/project/<tag or branch>/dist/install.yaml and kubectl apply -k https://github/<org>/project/<tag or branch>/config/default?
The gain I can think of is that the common kubernetes users don't need to study the kustomize grammar and a bundled YAML is very straightforward.

varshaprasad96
varshaprasad96 approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Camila! This is a great addition!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, Kavinjsir, varshaprasad96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [camilamacedo86,varshaprasad96]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@varshaprasad96
Copy link
Member

Looks like unit tests a failing. Would have to dig deeper, probably import issues, or version bumps in ginko?

@camilamacedo86
Copy link
Member Author

@varshaprasad96

The unit tests are failing for unrelated reason, it was fixed here: #3743 (coverage test bumped with go 1.19 instead of 1.20)
I closed and re-opened the PR to see if it will get master changes. If not, I just rebased, and we are done.

Really thank you for your help and support 🥇

@k8s-ci-robot k8s-ci-robot merged commit 1e495ea into kubernetes-sigs:master Jan 26, 2024
33 of 34 checks passed
@camilamacedo86 camilamacedo86 deleted the add-distro-mode branch January 26, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varshaprasad96 varshaprasad96 varshaprasad96 approved these changes

@Kavinjsir Kavinjsir Kavinjsir approved these changes

@rashmigottipati rashmigottipati Awaiting requested review from rashmigottipati

@everettraven everettraven Awaiting requested review from everettraven

Assignees

@varshaprasad96 varshaprasad96

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide propose solution to add a helper for kubebuilder users distribute their solutions
5 participants
@camilamacedo86 @dashanji @k8s-ci-robot @varshaprasad96 @Kavinjsir